Conversation
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces a substantial cross-cutting rewrite of Swift iOS synchronization, lifecycle, thread reconciliation, and Stop behavior across production paths, rather than a narrowly isolated fix. Unresolved correctness risks remain in Stop completion handling and shell hydration/connection-state recovery. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
@t3dotgg Alex has approved this PR for your review. Stop feedback persists until the session settles, including pending-question controls. The current description contains the verification evidence. |
5e0055b to
b0a4aa4
Compare
| @@ -1340,6 +1529,7 @@ public final class FeatureRootModel { | |||
| pendingRewindRecoveryIDs.remove(thread.id) | |||
| rewindErrors[thread.id] = nil | |||
| } | |||
| for thread in value.threads { reconcileStop(thread) } | |||
There was a problem hiding this comment.
🟠 High Root/FeatureRootModel.swift:1532
An idle snapshot leaves a successful Stop request in .awaitingOutcome, so the thread remains non-actionable and its Stop-related controls stay disabled indefinitely. install now calls reconcileStop for every snapshot, but that reconciliation rejects the valid inactive idle status even though the native mapper represents it as a completed latest turn; update reconcileStop to finalize the request for idle snapshots as well.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/Features/Root/FeatureRootModel.swift around line 1532:
An `idle` snapshot leaves a successful Stop request in `.awaitingOutcome`, so the thread remains non-actionable and its Stop-related controls stay disabled indefinitely. `install` now calls `reconcileStop` for every snapshot, but that reconciliation rejects the valid inactive `idle` status even though the native mapper represents it as a completed latest turn; update `reconcileStop` to finalize the request for `idle` snapshots as well.
A detached accepted-command refresh could finish after deleteThread and publish the deleted thread again. Cancel it once the server accepts the delete. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
b0a4aa4 to
cfb8289
Compare
| return true |
There was a problem hiding this comment.
🟠 High App/NativeFeatureClient.swift:1202
The same-client fast path preserves the previous shell epoch, so after a stream restart acceptActiveShell rejects the lower-sequence HTTP hydration and the UI remains on stale projects and threads. Update activeShellConnectionID and activeShellEpochHasSnapshot before returning from this path, as in the replacement-client path.
- return true
+ activeShellConnectionID = adoptedConnectionID
+ activeShellEpochHasSnapshot = adoptedConnectionID != nil
+ && shellConnectionIDsByEnvironmentID[environment.id] == adoptedConnectionID
+ return true🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 1202:
The same-client fast path preserves the previous shell epoch, so after a stream restart `acceptActiveShell` rejects the lower-sequence HTTP hydration and the UI remains on stale projects and threads. Update `activeShellConnectionID` and `activeShellEpochHasSnapshot` before returning from this path, as in the replacement-client path.
…ng older state A detail read that started before a newer send was accepted could publish after it and hide the newer message until the next read. Skip publishing a superseded read (its refresh loop reads again), and stop a cancelled shell refresh from writing a shell read before the change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ives A Stop accepted during a send's detail read discarded that read without requesting another, and an older detail could replace the transcript before the accepted message reached it. Re-read detail after a discarded read, and keep delivered messages until a server transcript includes them. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Do not retain a delivered message that a server transcript already confirmed, keep only its display copy, and drop retained messages when their thread details are removed or cleared. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
cfb8289 to
22a2817
Compare
Evicting a cached thread detail forgot its accepted messages, so a stale reopen hid them. A restored outbox or a new task's local detail was also recorded as the server transcript, so its accepted prompt was treated as confirmed and disappeared on the next older read. Only server transcripts now confirm delivery, and eviction keeps retained messages. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
22a2817 to
edb3c12
Compare
A newer send can be delivered while an older one waits to retry. Delivered copies were placed before every queued copy, which reversed them. Merge both by send time. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A transient environment catalog read failure ended the active client's configuration subscription, so later provider and settings updates were ignored. Skip that publish instead. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…e probe A passive peer's catalogue probe started at bootstrap could finish after the user saved a preference there and replace it with the older value. Discard the probe when a config already arrived. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
edb3c12 to
3595ca0
Compare
A peer whose credential was rejected before its catalogue loaded kept opening a probe client every 20 seconds. The probe's own error cannot show the rejection (the socket ticket 401 surfaces as a timeout), so stop once the peer's shell read has marked it as needing pairing; re-pairing starts a new worker. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ismissed early Reopening a thread in the compact split view can report the new thread view as disappeared right after it appears while it stays on screen. That cancelled the draft restore, so the composer showed a busy send button until relaunch. Restore outside the view task's lifetime. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…cted Pairing rejection cancelled the polling tasks but left the reconciliation loop issuing rejected shell reads every 30 seconds. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A passive peer whose socket stayed open kept its 30-second quiet shell reads after a 401, and showed as connected. Handle the rejected credential before the live-stream shortcut and switch to the long back-off. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The open socket's updates marked a rejected peer connected again, and stream repairs woke its HTTP loop before the back-off. Keep the rejection in the peer's shared state. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The live-peer fixture this test needs now lands below the Stop layer, so the regression moved out of the command-acknowledgement layer returns here.
Stop status refreshes the shell cache and reads it back. A live turn advancing the shell during that read made the refresh discard itself as superseded, so the status check failed and Stop could not be confirmed or retried. While the environment's stream is authoritative on the current socket, the cache it keeps current answers instead; after a disconnect or lost authority the check still fails. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
3595ca0 to
33c35dd
Compare
| guard let activeEnvironment else { return } | ||
| publish(makeSnapshot( | ||
| environments: environments, activeEnvironment: activeEnvironment, | ||
| connectionState: connectHeader ? .connected : (latestSnapshot?.connection.state ?? .reconnecting), |
There was a problem hiding this comment.
🟡 Medium App/NativeFeatureClient.swift:454
A successful fallback HTTP hydration applies the shell but leaves the published root snapshot in .disconnected when the previous hydration failed and no WebSocket snapshot has arrived. This preserves latestSnapshot.connection.state because connectHeader is false, so consumers continue to treat the reachable session as offline; publish the successful shell with a connected state instead.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/swift-ios/App/NativeFeatureClient.swift around line 454:
A successful fallback HTTP hydration applies the shell but leaves the published root snapshot in `.disconnected` when the previous hydration failed and no WebSocket snapshot has arrived. This preserves `latestSnapshot.connection.state` because `connectHeader` is false, so consumers continue to treat the reachable session as offline; publish the successful shell with a connected state instead.
There was a problem hiding this comment.
Acknowledged: after a failed hydration, a later HTTP-only hydration applies the shell but can leave the root connection state at disconnected until the socket snapshot arrives. Listed as a follow-up.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
|
@t3dotgg this is ready for your review and merge. Merge order: stacked chain, merge in order: #10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #10767. #12655 (composer focus freeze) is independent and can merge at any time. Evidence at head
Independent review: GPT-6 Astra xhigh (read-only) reviewed the Stop-status fix in this layer across rounds 8–12. It rejected two broader versions; the final version returned ready with no medium or higher findings. The rest of the layer was covered by the SWE-2 Max review of the whole chain. Macroscope findings on the latest heads have been verified against the source: real ones are fixed, and the rest have a written reason and are listed under Follow-ups in the description. |
|
Note This comment is posted by Julius' dot The new Stop unconfirmed and Retry stop workflow changes cancellation behavior across navigation and reconnects. That scope requires prior approval. The comment mentioning Alex’s approval does not link maintainer direction. Please link that approval or agree on the Stop workflow with maintainers, then request reconsideration. |
2 participants
Review this layer
GitHub shows this PR against
t3code/rebuild-mobile-app-swift, so its diff includes every earlier PR in the chain. Review only this layer's change:This layer's diff: +4060 / −25 lines across 11 files; app code +238 / −16, the rest is tests and docs.
An accepted Stop request is not a terminal session outcome. Retain Stopping feedback across thread navigation, expose an unconfirmed state when the outcome cannot be verified, and check current status before Retry stop dispatches again. Keep repeated taps from duplicating an outstanding stop. Disable pending question controls while Stop is active, and resume other queued submissions even when Stop fails.
Verification: current-head CI passes, including contract fixtures and native tests. The final bootstrap/environment run passed 38 tests, covering environment switching, error visibility and archive hydration; separate focused checks cover passive-worker lifetime. The Stop regression run passed 119 tests; native before/after captures verify pending question controls are disabled until Stop completes. Skipped checks are not claimed as executed proof.
Delivery: stacked. This branch contains #10758, #10759, #10761, #10762, #10763, #10764; merge the chain in order (#10758 → #10759 → #10761 → #10762 → #10763 → #10764 → #10765 → #10766 → #14021 → #10767). Each PR's own change is its top commit(s).
Base: Theo’s
t3code/rebuild-mobile-app-swift,157476f1fb.Visual verification: Baseline
a361b2212and candidate9bcc08beause actual ThreadDetailView. Both captures passed and assert one Stop dispatch. The candidate retains unconfirmed feedback after acknowledgement and clears it after completed status. The fixture supplies no scoped connection, so it correctly does not claim a confirmed remote outcome. Native model tests separately cover connected acknowledgement, close/reopen retention, duplicate taps and status reconciliation before retry.Actual before/after stills, held for 3.5 seconds each; not a motion or latency measurement.
Before screenshot · After screenshot
Before (base), full recorded sequence at 12 fps and real-time playback:
After (candidate), full recorded sequence at 12 fps and real-time playback:
Before video · Clean candidate video · Annotated candidate video · Evidence receipt
Additional Stop regression proof: baseline
faa5574e6leaves Submit actionable during an outstanding Stop. Candidate58ad6fe56disables the answer panel until completion. Both native captures passed; semantic snapshots confirm controls become available again after completion, and no answer was sent. The fixture invokes the actual model Stop method after selecting Continue; it does not depict a Stop-button tap. A separate regression verifies failed Stop resumes other queued submissions.Actual before/after stills, held for 3.5 seconds each; not a motion or latency measurement.
Before screenshot · After screenshot
Before (base), full recorded sequence at 12 fps and real-time playback:
After (candidate), full recorded sequence at 12 fps and real-time playback:
Before video · Clean candidate video · Annotated video · Evidence receipt
Capture currency: checked against
5e0055bdc. The original Stop scenario contains no pending question or queued creation, so later changes to those paths do not alter that demonstration. The separate pending-question capture at58ad6fe56includes the final control fix; only environment lookup error propagation and its test changed afterward.Visual review: inspected the before/after images and recorded interaction states at 390 px width against current head
5e0055bdc. The demonstrated UI is unchanged from the labeled capture revisions; retained content, recovery controls, spacing, and text are legible. This is the author’s visual assessment, not maintainer approval.Independent cross-provider review was unavailable:
claude auth statusreturned exit 1 withloggedIn: false. No Claude review or maintainer approval is claimed.Refresh onto
157476f1fb(2026-09-27)Rebased with the thread-sync chain onto the current
t3code/rebuild-mobile-app-swift(157476f1fb). Head33c35dd32f.NativeMultiEnvironmentTests,NativeThreadCatchUpTests,NativeRetryIdentityTests,FeatureRootModelTests, iOS Simulator, per-test time limits): 117 passed.T3CodeTestsat the chain top (68d92c2752): 343 XCTest cases passed (1 skipped). Swift Testing ran 892 tests; the only 2 failures areUsageModelsTestscurrency formatting, which fail identically on unmodified157476f1fb.devin -p --sandbox --permission-mode smart --model swe-2-maxover the whole chain diff, exit 0. No blocker, high or medium findings. A dead-state finding was fixed. A report that Stop state outlives a vanished thread was not changed, becausestopSurvivesCloseReopenAndMissingReconnectSeedrequires that behaviour.Review fixes (round 7: Macroscope and Astra)
stopStatusDuringLiveUpdates(environmentID:authorityLost:)(cases active superseded, active authority lost, peer superseded). The original code fails the superseded cases.codex exec -m gpt-6-astra -c model_reasoning_effort="xhigh" --sandbox read-only) reviewed this fix across rounds 8–12. It rejected two broader versions: one that trusted the cache after a reconnect, and one that answered from the HTTP read alone, which could overwrite newer visible state and leavecancelTurnvalidating a stale cache. The final version returned ready with no medium or higher findings.Follow-ups (minor, not blocking)
Model and harness: GPT-6 / Codex. Rebase onto
157476f1fb: Claude Opus 5.5 / Claude Code.